-
Notifications
You must be signed in to change notification settings - Fork 411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add link_deps field #879
Add link_deps field #879
Conversation
What would the combined argument look like? Is it the idea of writing
We do allow custom link flags for libraries, and some of them are passed to the C compiler, so I guess it would make sense as well to have link dependencies for libraries. Although it's probably much less common. The fields for libraries are named differently as well:
So I guess the name for
Yes. This code would need to be refactored so that we can use it for flag lists as well. |
Sorry, that wasn't very clear but I was referring to the arguments passed into Instead of adding an extra argument to that function (and other ones in
Would that be preferable? |
Ah, I see. Yes that makes sense. In fact, we could allow to use the special syntax |
@emillon would you mind updating CHANGES and the manual btw? |
|
You need to add a
Nope, that's the only place
Sure |
In some cases, the linking step requires some dependencies. For example, passing a version script to the linker. The new `(link_deps)` field uses the dependency DSL already used in other places. Closes ocaml#852 Signed-off-by: Etienne Millon <etienne@cryptosense.com>
Oops, forgot to |
Looks good |
Thanks! |
Hi!
Description
In some cases, the linking step requires some dependencies (see #852). For example, passing a version script to the linker. The new
(link_deps)
field uses the dependency DSL already used in other places.Questions
While working on this PR with @NathanReb we noted some questions about the implementation:
link_deps
be an extra argument, or should it be embedded intolink_flags
? (in the latter case, maybe we can rename the combined argument).exe
the correct place to put this, or should it be onBuildable.t
? (it seems that libraries are handled completely by the OCaml toolchain, so there are no external dependencies)%{dep:path}
syntax? it seems that the magic to do that is inSuper_context.Action.expand_step1
, but this would apply to all places where the action syntax is supported. Is this what you meant?Thanks!